Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recommend changes to cluster weapon randomness #3929

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

efrec
Copy link
Contributor

@efrec efrec commented Nov 13, 2024

Work done

I want to give the scatter of cluster projectiles more of a natural look. One weapon in particular I didn't realize was barely producing random spreads, since the direction set it was pulling from was poorly distributed. This correction applies to the Eviscerator the most after fixing that direction set.

I gave cluster projectiles a nudge in the direction of the parent projectile's travel, also. This doesn't end up pushing them forwards but decreases the kind of odd appearance when they go backwards against this direction due to randomness. A larger push would be close to a balance change, but I don't think this affects anything at all.

First commits are to clean up my earlier, poorer code. The rest shows what changes were made and why.

efrec added 15 commits November 12, 2024 15:20
This is a lot of small changes adding up to the same code. The small emphasis towards the primary projectile's direction only really shows in that cluster munitions never seem to bounce "backwards" anymore (they sometimes did).
this was previously done with a sort of 'boquet-ing' pattern of setting the y-up velocity too high, but this just causes projectiles to linger in the air for very long and doesn't look natural at all
earlier commit introduced randomness to the projectile velocity which further reduced the actual travel distance, this fixes
Code doesn't need to be this opinionated. Multiple primary weapons can use the same cluster def now with better control over the result.
Copy link
Member

@WatchTheFort WatchTheFort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a lot of magic numbers in the code, where are they coming from? Were the values all chosen empirically?

Comment on lines +27 to +28
-- This set is too axis-aligned, so I'm rotating it. The main issue is the vertical y-down, which tends to shoot into the ground.
-- { 0.212548255920, -0.977150570601, 0.000000000000, -0.977150570601, -0.212548255920, 0.000000000000, -0.212548255920, 0.977150570601, 0.000000000000, 0.977150570601, 0.212548255920, 0.000000000000, 0.000000000000, 0.000000000000, 1.000000000000, 0.000000000000, 0.000000000000, -1.000000000000 },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a need for this comment or keeping the commented-out code? This is what the commit and PR messages are for.

local speedRatio = min(1, tonumber(custom.cluster_speed_ratio or -1))

local clusterDefName = custom.cluster_def
if not clusterDefName or not WeaponDefNames[clusterDefName] then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does an undeclared cluster_def need to be supported?

search = search == '' and word or search .. '_' .. word
if UnitDefNames[search] ~= nil then
dataTable[wdid].def = search .. '_' .. defaultSpawnDef
local dataTable = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
local dataTable = {}
local clusterWeaponDefs = {}

More descriptive name

Spring.Log(gadget:GetInfo().name, LOG.ERROR, 'Preventing nested explosions: '..WeaponDefs[weaponDefID].name)
dataTable[weaponDefID] = nil
end
removeIDs = nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? Lua garbage collection will handle disposal.

Comment on lines +155 to +159
local bulkiness = (
unitDef.health ^ 0.5 + -- HP is log2-ish but that feels too tryhard
unitDef.metalCost ^ 0.5 * -- Steel (metal) is heavier than feathers (energy)
unitDef.xsize * unitDef.zsize * unitDef.radius ^ 0.5 -- We see 'bigger' as 'more solid' not 'less dense'
) / minBulkReflect -- Scaled against some large-ish bulk rating
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just an analog for the unit's mass?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caveat, native engine mass does a bunch of things. So tweaking it for pathing might ruin its behaviour with cluster weapons and vice versa, if the same value is used here. beyond-all-reason/spring#526

@WatchTheFort
Copy link
Member

@efrec Have you had a chance to look at the comments/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants